Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for document data source #161

Closed
wants to merge 7 commits into from
Closed

Add support for document data source #161

wants to merge 7 commits into from

Conversation

sdahlbac
Copy link
Contributor

@sdahlbac sdahlbac commented Mar 20, 2024 •
edited
Loading

I have have only tested this with unittests and cli, as I do not have a connect.

  • Supports multiple files
  • Supports text documents and binary documents (provided one uses the content_base64 version)

Roundtrip test:

data "onepassword_item" "foo" {
  vault = var.demo_vault
  uuid  = "..."
}
resource "local_file" "truststore" {
  filename       = "truststore.jks"
  content_base64 = data.onepassword_item.foo.file.2.content_base64
}

(where 2 is the third file for that item, added via the UI)

I had to use the op read approach rather than op document get (unless I am missing an undocumented feature) to be able to handle multiple files.

I have not tried to implement a resource, since as far as I understood connect cannot support it and I don't see a way to distinguish between the two backends.

The document category is available also for resource, that is slightly incorrect.

@volodymyrZotov volodymyrZotov self-requested a review April 2, 2024 16:18
@estenrye
Copy link

estenrye commented Apr 4, 2024

This looks related to #51. I have a connect API setup for my Kubernetes clusters, I might be able to test this if I get some time to build and deploy your branch.

@sdahlbac sdahlbac marked this pull request as ready for review April 4, 2024 19:47
volodymyrZotov
volodymyrZotov reviewed Apr 17, 2024
View reviewed changes
onepassword/data_source_onepassword_item.go Outdated Show resolved Hide resolved
onepassword/data_source_onepassword_item_test.go Outdated Show resolved Hide resolved
onepassword/resource_onepassword_item.go Outdated Show resolved Hide resolved
@sdahlbac
Copy link
Contributor Author

sdahlbac commented Apr 18, 2024 •
edited
Loading

Ran go generate to get updated docs, but it seems that terraform-plugin-docs fails to include descriptions of nested fields 😢 EDIT: turns out that the descriptions are already missing in terraform providers schema -json

@sdahlbac
Copy link
Contributor Author

@volodymyrZotov comments adressed

@sdahlbac
Copy link
Contributor Author

Speculating with myself: Actually, the fact that descriptions are missing from the schema might be a slight issue if things like sensitive: true is also missing

@sdahlbac
Copy link
Contributor Author

sdahlbac commented May 16, 2024 •
edited
Loading

It seems that the cheese has been moved (due to #167). I'll see if I can figure out what changes are needed due to the change of tf framework.

Also, it seems that #164 is somewhat related.

@sdahlbac
Copy link
Contributor Author

Ok, I have something that seems to work locally against main. Will create a superseding PR when I have cleaned up it a little bit more.

@sdahlbac sdahlbac mentioned this pull request May 17, 2024
Merged
@sdahlbac
Copy link
Contributor Author

Superceded by #171

@volodymyrZotov would you review that one instead, please?

@sdahlbac sdahlbac closed this May 17, 2024
@volodymyrZotov
Copy link
Collaborator

volodymyrZotov commented May 17, 2024 •
edited
Loading

@sdahlbac Hi, was about to write to you that there was a migration and you would need to adjust your PR because of that, but I see you figured it out already 👍 Will take another look there.

This was referenced Jun 17, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@volodymyrZotov volodymyrZotov volodymyrZotov left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sdahlbac @estenrye @volodymyrZotov